-
-
Notifications
You must be signed in to change notification settings - Fork 16
[18.0][MIG] hr_shift: Migration to version 18.0 #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
TT50623 Co-authored-by: chienandalu [UPD] Update hr_shift.pot [BOT] post-merge updates
Changes done: - Add rules related to shift lines - Add My shifts filter + group by fields - Add template + employee fields to calendar view - Set widget="many2one_avatar_employee" to employee_id field (tree view)
Co-authored-by: neitherkx <dpalancamartinez@gmail.com>
… a template without error (similar to v14)
|
Also @mathben can you check if your issue was fixed? |
|
@david-banon-tecnativa Thanks, my problem is solved! I propose this little check to reduce crash when missing data. |
|
|
||
| onWillStart(async () => { | ||
| this.isHrOfficer = await this.user.hasGroup("hr.group_hr_user"); | ||
| this.showGeneratePlanning = this.isHrOfficer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
showGeneratePlanning need to be false if has no records, the wizard is only useful when contain records, because it needs to copy from existing.
I try something like that, but it's not working
Object.defineProperty(this, "hasRecords", {
get: () => {
const root = this.model?.root;
const count = root?.count ?? root?.records?.length ?? 0;
return count > 0;
},
});
onWillStart(async () => {
this.isHrOfficer = await this.user.hasGroup("hr.group_hr_user");
this.showGeneratePlanning = this.isHrOfficer && this.hasRecords;
});There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What crash are you talking about? What steps need to be taken to reproduce it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@david-banon-tecnativa hide the button «Generate planning» on top when has no data, because it needs an old planning to generate a new one. The problem occur at first installation with no demo, no data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can merge this pull request and fix this later, it's not a blocking bug, just a wrong user case.
eduezerouali-tecnativa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@david-banon-tecnativa code LGTM + runboat test. Just one thing we have two mig to 18.0 commits, shouldn't they be squash?
|
Let's go with it and it can be improved later. Thanks to all the involved. /ocabot migration hr_shift |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at c73be25. Thanks a lot for contributing to OCA. ❤️ |
| ) | ||
| shift.line_ids.create(shift_lines) | ||
|
|
||
| def create(Self, vals_list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is Self a typo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, thanks for find it out, however as it is merged and it doesn't cause any trouble, we can leave as is, and it can be fixed when the module receives another change.
Supersedes #17
Additional work done:
@Tecnativa @victoralmau @eduezerouali-tecnativa
TT57629